Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the replay tile’s handling of FEC ingestion and bank-frontier eviction to prevent “stem burst” violations by bounding the amount of work performed per after_credit iteration.
Changes:
- Change
insert_fec_setto return anintstatus so callers can stop backfill once a bank is marked dead. - Track frontier eviction victims in
fd_replay_tile_t(victim_bank_indices/victim_bank_cnt) and evict them incrementally instead of in a single loop. - Update backfill logic in
process_fec_setto stop inserting further FEC sets when an insertion marks the bank dead.
Comments suppressed due to low confidence (1)
src/discof/replay/fd_replay_tile.c:2061
insert_fec_setnow returnsintbut can fall through to the end of the function without returning a value (successful ingest path after the store lock). This is undefined behavior and will typically trigger-Wreturn-type. Ensure the function returns a value on all paths (e.g., return 0 on the non-dead/success path after the lock block).
} FD_STORE_SLOCK_END;
ctx->metrics.store_query_release++;
fd_histf_sample( ctx->metrics.store_query_work, (ulong)fd_log_wallclock() - work );
}
You can also share your feedback on Copilot code review. Take the survey.
yufeng-jump
force-pushed
the
yufeng/eviction-burst
branch
from
5bffa5f to
5339cc0
Compare
yufeng-jump
force-pushed
the
yufeng/eviction-burst
branch
from
5339cc0 to
dc04011
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to mitigate replay “stem burst violations” by spreading out high-cost eviction/ingestion work across credits and by stopping backfill ingestion early once a block is detected as dead.
Changes:
- Make
insert_fec_set()return a status (dead vs not-dead) and stop backfill ingestion once a block is marked dead. - Replace “evict all frontier banks in one credit” with a deferred victim list (
victim_bank_indices/victim_bank_cnt) and evict one victim per credit inafter_credit(). - Initialize new replay-tile eviction state (
victim_bank_cnt) during unprivileged init.
You can also share your feedback on Copilot code review. Take the survey.
src/discof/replay/fd_replay_tile.c
Outdated
| fd_txncache_t * txncache; | ||
| fd_store_t * store; | ||
| fd_banks_t banks[1]; | ||
| ulong victim_bank_indices[ FD_BANKS_MAX_BANKS ]; |
Contributor
There was a problem hiding this comment.
can you call this something else
yufeng-jump
force-pushed
the
yufeng/eviction-burst
branch
from
dc04011 to
803489f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.